Fix Beacon API JSON envelope for blobs endpoint#54
Merged
Conversation
Collaborator
✅ Heimdall Review Status
|
BrianBland
reviewed
May 11, 2026
Comment on lines
+397
to
+402
| var responseBody map[string]json.RawMessage | ||
| err = json.Unmarshal(response.Body.Bytes(), &responseBody) | ||
| require.NoError(t, err) | ||
| require.Contains(t, responseBody, "execution_optimistic") | ||
| require.Contains(t, responseBody, "finalized") | ||
| require.Contains(t, responseBody, "data") |
There was a problem hiding this comment.
I would personally default to skipping this sort of test case, as it's mostly redundant with the beaconBlobsResponse unmarshaling below
Comment on lines
+486
to
+491
| var responseBody map[string]json.RawMessage | ||
| err = json.Unmarshal(response.Body.Bytes(), &responseBody) | ||
| require.NoError(t, err) | ||
| require.Contains(t, responseBody, "execution_optimistic") | ||
| require.Contains(t, responseBody, "finalized") | ||
| require.Contains(t, responseBody, "data") |
Comment on lines
+223
to
+227
| type beaconBlobSidecarsResponse struct { | ||
| ExecutionOptimistic bool `json:"execution_optimistic"` | ||
| Finalized bool `json:"finalized"` | ||
| Data []*deneb.BlobSidecar `json:"data"` | ||
| } |
There was a problem hiding this comment.
Style: avoid interleaving struct definitions with the rest of the code.
In this case I'd just move this to the top of the file
| ExecutionOptimistic bool `json:"execution_optimistic"` | ||
| Finalized bool `json:"finalized"` | ||
| Data v1.Blobs `json:"data"` | ||
| } |
BrianBland
approved these changes
May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /eth/v1/beacon/blobs/{id}instead of encoding the blobs array directly.execution_optimistic,finalized, anddatafields for JSON responses.application/octet-streamstill returns the raw serialized blob list.Context
The PeerDAS/Fulu support added a
/eth/v1/beacon/blobs/{block_id}route as the fallback-compatible blob endpoint. The handler converted archived blob sidecars intov1.Blobs, but encoded that slice directly for JSON responses.That was not Beacon API-compatible. Beacon API JSON responses wrap returned payloads under a top-level
datakey, and thegetBlobsresponse schema also includesexecution_optimisticandfinalized. SSZ is the format that returns raw serialized blob-list bytes.This broke op-node fallback compatibility because op-node calls
/eth/v1/beacon/blobs/{slot}and decodes the response intoeth.APIBeaconBlobsResponse, which expectsdatato contain the blob array. A top-level JSON array cannot decode into that wrapper type.The original tests missed this because they unmarshaled the response directly into
v1.Blobs, which matched the incorrect implementation rather than the Beacon API response shape.Testing
go test ./...